Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update security of Dockerfile #18

Merged
merged 4 commits into from
Aug 9, 2018
Merged

Update security of Dockerfile #18

merged 4 commits into from
Aug 9, 2018

Conversation

jstuder-gh
Copy link
Contributor

  • Use https to retrieve archive from Rakudo server
  • Retrieve PGP signature from Rakudo server (https)
  • Retrieve PGP public key over hkps and using full fingerprint
  • Verify archive using signature (explicitly using gpg2)

@jstuder-gh
Store all intermediate files in temp dir
048e15a 
Makes for easy cleanup.
@jstuder-gh
Upgrade security of Dockerfile
33cba95 
* Use https to retrieve archive from Rakudo server
* Retrieve PGP signature from Rakudo server (https)
* Retrieve PGP public key over hkps and using full fingerprint
* Verify archive using signature (explicitly with gpg2)
tianon
tianon reviewed Aug 9, 2018
View reviewed changes
Dockerfile Outdated
@@ -12,19 +12,29 @@ RUN buildDeps=' \
libencode-perl \
make \
' \
url="https://rakudo.org/downloads/star/rakudo-star-${rakudo_version}.tar.gz" \
keyserver='hkps.pool.sks-keyservers.net' \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As written, this won't actually use hkps (it'll use hkp against the hkps pool, which will generally work), but I'd also argue that when using a full fingerprint, hkps doesn't provide additional benefit since gpg will verify the fingerprint of the key after fetching which is a stronger guarantee, and not using hkps allows us to intentionally mitm for increased reliability (see docker-library/php#666), which we do on all the official build servers for the official images.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the insight! I'll update the branch to use the 'ha' pool as suggested on the official image readme.

@jstuder-gh
Use "High Availabilty" Keyserver Pool
91de061 
Use the 'ha' keyserver pool as...
  * GPG already provides strong guaranty when using a full fingerprint
  * Using hkps reduces reliability enhancements Docker provides for
    official images
Thanks to tianon++ for the review.
See [here](https://github.com/perl6/docker/pull/18/files#r208803260)
for more details.
@jstuder-gh
Copy link
Contributor Author

@tianon, I noticed that the Travis build is failing with gpg2: not found, which surprises me as it builds just fine for me on my local machine (it should be using the same base image, right?)

Would it be best to use plain 'gpg' here? Given the base image, I believe gpg is an alias for gpg2, but if not and GPG v1.* were used would that greatly affect security in this instance? Thanks.

@zakame
Copy link
Member

zakame commented Aug 9, 2018 •
edited
Loading

@jstuder-gh gpg on buildpack-deps should already be gpg2:

θ69° [zakame:~] % docker run --rm -it buildpack-deps:stretch /bin/sh
# which gpg2
# dpkg -s gnupg | grep Version
Version: 2.1.18-8~deb9u1
# which gpg
/usr/bin/gpg
# gpg --version
gpg (GnuPG) 2.1.18
libgcrypt 1.7.6-beta
Copyright (C) 2017 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <https://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

Home: /root/.gnupg
Supported algorithms:
Pubkey: RSA, ELG, DSA, ECDH, ECDSA, EDDSA
Cipher: IDEA, 3DES, CAST5, BLOWFISH, AES, AES192, AES256, TWOFISH,
        CAMELLIA128, CAMELLIA192, CAMELLIA256
Hash: SHA1, RIPEMD160, SHA256, SHA384, SHA512, SHA224
Compression: Uncompressed, ZIP, ZLIB, BZIP2

So better use gpg in the Dockerfile RUNs.

@jstuder-gh
Use 'gpg' as opposed to explicit 'gpg2'
944167b 
Travis was failing due to being unable to find 'gpg2' and gpg is already
an alias for gpg2 in the base image. Thanks to zakame++ for
verification.
@jstuder-gh
Copy link
Contributor Author

Thanks @zakame for confirming on your end. I've updated the branch accordingly.

@hoelzro
Copy link
Contributor

hoelzro commented Aug 9, 2018

If all looks good to @tianon, I can merge this!

@tianon
Copy link
Contributor

tianon commented Aug 9, 2018

Looks great to me! 👌

The gpg2 discrepancy was likely just an outdated base image; doing docker build --pull ... would probably help/fix that. 👍

@hoelzro
Copy link
Contributor

hoelzro commented Aug 9, 2018

Ok, thanks @jstuder-gh for the contribution, and thanks to @tianon and @zakame for reviewing!

@hoelzro hoelzro merged commit 8d988c4 into Raku:master Aug 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tianon tianon tianon left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@jstuder-gh @zakame @hoelzro @tianon @georgy7